Extract mixin from PV and EV component managers#1403
Conversation
github-actions
Bot
added
the
part:microgrid
simonvoelcker
added
the
cmd:skip-release-notes
simonvoelcker
force-pushed
the
extract-set-power
branch
2 times, most recently
from
4b9e90f to
e28fc25
Compare
simonvoelcker
requested review from
ela-kotulska-frequenz
and removed request for
a team
llucax
left a comment
llucax
left a comment
There was a problem hiding this comment.
LGTM. I wonder if it makes more sense to just include it in ComponentManager and make BatteryManager override it if it needs something special, instead of introducing a separate mixin.
Will leave @shsms the final approval as he's more familiar with this code.
|
@llucax I was considering that, but the battery manager is structured very differently. The closest match to the extracted method, |
shsms
left a comment
shsms
left a comment
There was a problem hiding this comment.
We use relative import paths, and one optional comment.
| """Mixin for setting component powers via microgrid API.""" | ||
|
|
||
| @staticmethod | ||
| async def _set_api_power( # pylint: disable=too-many-locals,too-many-arguments |
There was a problem hiding this comment.
Optional, but I wonder if this should just be a function instead, the mixin is a lot of machinary for a static method that doesn't need self.
github-project-automation
Bot
moved this from To do
to Review approved
in Python SDK Roadmap
|
looks like CI is failing |
simonvoelcker
force-pushed
the
extract-set-power
branch
from
8102ec8 to
995f835
Compare
PV inverter manager, EV charger manager and also battery manager have similar functionality for setting power levels in the microgrid API. These have drifted apart to different degrees, especially the battery manager implementation differs a lot. The other two were close enough to extract them into a mixin. This will be reusable when we introduce a similar manager for steam boilers. Signed-off-by: Simon Völcker <simon.voelcker@frequenz.com>
simonvoelcker
force-pushed
the
extract-set-power
branch
from
995f835 to
f9ab525
Compare
github-project-automation
Bot
moved this from Review approved
to Done
in Python SDK Roadmap
3 participants
PV inverter manager, EV charger manager and also battery manager have similar functionality for setting power levels in the microgrid API. These have drifted apart to different degrees, especially the battery manager implementation differs a lot. The other two were close enough to extract them into a util function. This will be reusable when we introduce a similar manager for steam boilers.